Skip to content

[SCAL-327613] Add Spotter routes to fullHeight height-reset allowlist - #652

Closed
shikharbsar wants to merge 1 commit into
mainfrom
SCAL-327613
Closed

shikharbsar wants to merge 1 commit into
mainfrom
SCAL-327613

Conversation

@shikharbsar

Copy link
Copy Markdown
Contributor

Spotter (conv-assist) routes were missing from the RouteChange allowlist that skips the fullHeight iframe height reset, causing the embed to collapse to defaultHeight when navigating into or within Spotter.

  • Add /insights/conv-assist and /embed/insights/conv-assist to liveboardRelatedRoutes in app.ts and liveboard.ts
  • Add pinning tests for the bare landing route and the conversation-id URL variant

@shikharbsar
shikharbsar requested a review from a team as a code owner September 2, 2026 19:25

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request adds Spotter (conv-assist) routes to the liveboardRelatedRoutes array in both AppEmbed and LiveboardEmbed to prevent resetting the iframe height during navigation, along with corresponding unit tests. The review feedback highlights a potential partial prefix matching bug due to the lack of trailing slashes on the new routes, and suggests extracting the duplicated route array and matching logic into a shared constants file.

Comment thread src/embed/app.ts Outdated
Comment thread src/embed/liveboard.ts Outdated
@pkg-pr-new

pkg-pr-new Bot commented Sep 2, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/@thoughtspot/visual-embed-sdk@652

commit: de702ed

Spotter (conv-assist) routes were missing from the RouteChange
allowlist that skips the fullHeight iframe height reset, causing
the embed to collapse to defaultHeight when navigating into or
within Spotter.

- Add /insights/conv-assist and /embed/insights/conv-assist to
  liveboardRelatedRoutes in app.ts and liveboard.ts
- Add pinning tests for the bare landing route and the
  conversation-id URL variant
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants